Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a catch-all route if needed #2586

Merged
merged 8 commits into from
Feb 15, 2024

Conversation

zhaohuabing
Copy link
Member

fix: #2507

@zhaohuabing zhaohuabing requested a review from a team as a code owner February 12, 2024 02:31
@zhaohuabing zhaohuabing marked this pull request as draft February 12, 2024 02:31
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e4b88aa) 63.41% compared to head (c89643e) 63.57%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2586      +/-   ##
==========================================
+ Coverage   63.41%   63.57%   +0.16%     
==========================================
  Files         119      119              
  Lines       19163    19198      +35     
==========================================
+ Hits        12152    12206      +54     
+ Misses       6196     6180      -16     
+ Partials      815      812       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

// For filters without native per-route support, we need to add a catch-all route
// to ensure that these filters are disabled for non-matching requests.
// https://github.com/envoyproxy/gateway/issues/2507
func addCatchAllRoute(xdsIR map[string]*ir.Xds) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhaohuabing zhaohuabing marked this pull request as ready for review February 13, 2024 14:00
arkodg added a commit to arkodg/gateway that referenced this pull request Feb 14, 2024
Add a field called `useForRouting` that signals to Envoy Gateway
that the headers generated from the claims are used to make routing
decisions

Internally this field will be used to
* insert a catch-all route with a 404 direct response
identical to envoyproxy#2586 which
makes sure the jwt filter with `claimToHeader` is applied before
recomputing routing decision
* enable `clear_route_cache` to recompute routing decision
https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/jwt_authn/v3/config.proto#extensions-filters-http-jwt-authn-v3-jwtprovider

Relates to envoyproxy#2452

Signed-off-by: Arko Dasgupta <[email protected]>
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @zhaohuabing !

@arkodg arkodg requested review from a team February 15, 2024 03:23
@zhaohuabing zhaohuabing merged commit 35e646d into envoyproxy:main Feb 15, 2024
22 checks passed
vixns pushed a commit to vixns/gateway that referenced this pull request Feb 18, 2024
* add a catch-all route if needed

Signed-off-by: huabing zhao <[email protected]>

* fix lint and gen

Signed-off-by: huabing zhao <[email protected]>

* make catch all route name unique across mulitple hosts

Signed-off-by: huabing zhao <[email protected]>

* Update internal/gatewayapi/translator.go

Co-authored-by: Arko Dasgupta <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>

* address comments

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Signed-off-by: Stéphane Cottin <[email protected]>
zhaohuabing added a commit to zhaohuabing/gateway that referenced this pull request Feb 20, 2024
zhaohuabing added a commit that referenced this pull request Feb 20, 2024
…2663)

* Revert "add a catch-all route if needed (#2586)"

This reverts commit 35e646d.

* disable per-route filters at the http_filters

Signed-off-by: huabing zhao <[email protected]>

---------

Signed-off-by: huabing zhao <[email protected]>
arkodg added a commit to arkodg/gateway that referenced this pull request Feb 20, 2024
Add a field called `useForRouting` that signals to Envoy Gateway
that the headers generated from the claims are used to make routing
decisions

Internally this field will be used to
* insert a catch-all route with a 404 direct response
identical to envoyproxy#2586 which
makes sure the jwt filter with `claimToHeader` is applied before
recomputing routing decision
* enable `clear_route_cache` to recompute routing decision
https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/jwt_authn/v3/config.proto#extensions-filters-http-jwt-authn-v3-jwtprovider

Relates to envoyproxy#2452

Signed-off-by: Arko Dasgupta <[email protected]>
arkodg added a commit to arkodg/gateway that referenced this pull request Feb 23, 2024
Add a field called `useForRouting` that signals to Envoy Gateway
that the headers generated from the claims are used to make routing
decisions

Internally this field will be used to
* insert a catch-all route with a 404 direct response
identical to envoyproxy#2586 which
makes sure the jwt filter with `claimToHeader` is applied before
recomputing routing decision
* enable `clear_route_cache` to recompute routing decision
https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/jwt_authn/v3/config.proto#extensions-filters-http-jwt-authn-v3-jwtprovider

Relates to envoyproxy#2452

Signed-off-by: Arko Dasgupta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security policy from different route interferes authentication
2 participants